chore: sync upstream PR #8454 - Fix navigation bar overlay in edge-to-edge mode on Android#67
chore: sync upstream PR #8454 - Fix navigation bar overlay in edge-to-edge mode on Android#67riderx wants to merge 3 commits into
Conversation
Added navigation bar visibility handling and adjusted safe area calculations. Updated insets handling for improved layout compatibility.
📝 WalkthroughWalkthroughThe Android Capacitor plugin's system bars handling is refactored to improve navigation bar visibility tracking, insets workflow, and safe-area CSS injection. Changes include adding navigation bar visibility state, restructuring insets computation via parent views, enhancing safe-area calculations with keyboard awareness and webview version guards, and refining style application logic. ChangesSystem Bar and Insets Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
d6113cc to
6136d3c
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java (1)
353-359:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
navBarVisiblenot updated whenshow(BAR_STATUS_BAR)implicitly reveals the navigation bar
windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars())(line 354) encompasses both status and navigation bars. Whenbar = BAR_STATUS_BARand the navigation bar was previously hidden, the nav bar becomes physically visible, but the secondifblock (line 356) is skipped, leavingnavBarVisible = false. On pre-API 30 devices this causesgetNavBarHeightFromResources()to return0, so the fallback bottom safe-area inset will be under-reported despite the nav bar being on screen.🐛 Proposed fix
if (bar.isEmpty() || bar.equals(BAR_STATUS_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()); } if (bar.isEmpty() || bar.equals(BAR_GESTURE_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.navigationBars()); navBarVisible = true; }Either align the show path with the hide path (use
statusBars()forBAR_STATUS_BAR):if (bar.isEmpty() || bar.equals(BAR_STATUS_BAR)) { - windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()); + windowInsetsControllerCompat.show(WindowInsetsCompat.Type.statusBars()); } if (bar.isEmpty() || bar.equals(BAR_GESTURE_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.navigationBars()); navBarVisible = true; }Or, if retaining
systemBars()is intentional, also updatenavBarVisiblethere:if (bar.isEmpty() || bar.equals(BAR_STATUS_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()); + navBarVisible = true; // systemBars() restores navigation bar as well } if (bar.isEmpty() || bar.equals(BAR_GESTURE_BAR)) { windowInsetsControllerCompat.show(WindowInsetsCompat.Type.navigationBars()); navBarVisible = true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java` around lines 353 - 359, The show logic in SystemBars.java incorrectly leaves navBarVisible false when calling windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()) for BAR_STATUS_BAR because systemBars() reveals both status and navigation bars; update the navBarVisible state when showing systemBars() (set navBarVisible = true) or change the BAR_STATUS_BAR branch to call windowInsetsControllerCompat.show(WindowInsetsCompat.Type.statusBars()) instead so the navBarVisible flag remains correct for getNavBarHeightFromResources() and subsequent safe-area calculations; adjust the conditional handling around BAR_STATUS_BAR, BAR_GESTURE_BAR and the navBarVisible variable in the same method to keep hide and show paths symmetric.
🧹 Nitpick comments (1)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java (1)
166-182: ⚡ Quick winExtract duplicated window-flag setup into a shared helper
The window initialization block in
handleOnResume()(lines 170–177) is identical to the one insideinitSystemBars()(lines 116–123). If either copy drifts it will silently break one of the two code paths.♻️ Suggested refactor
+ private void applyEdgeToEdgeWindowFlags() { + Window window = getActivity().getWindow(); + WindowCompat.setDecorFitsSystemWindows(window, false); + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.VANILLA_ICE_CREAM) { + window.setNavigationBarColor(android.graphics.Color.TRANSPARENT); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + window.setNavigationBarContrastEnforced(false); + } + } + } private void initSystemBars() { ... getBridge().executeOnMainThread(() -> { - Window window = getActivity().getWindow(); - WindowCompat.setDecorFitsSystemWindows(window, false); - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.VANILLA_ICE_CREAM) { - window.setNavigationBarColor(android.graphics.Color.TRANSPARENT); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { - window.setNavigationBarContrastEnforced(false); - } - } + applyEdgeToEdgeWindowFlags(); setStyle(style, ""); setHidden(hidden, ""); ViewCompat.requestApplyInsets((View) getBridge().getWebView().getParent()); }); } `@Override` protected void handleOnResume() { super.handleOnResume(); getBridge().executeOnMainThread(() -> { - Window window = getActivity().getWindow(); - WindowCompat.setDecorFitsSystemWindows(window, false); - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.VANILLA_ICE_CREAM) { - window.setNavigationBarColor(android.graphics.Color.TRANSPARENT); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { - window.setNavigationBarContrastEnforced(false); - } - } + applyEdgeToEdgeWindowFlags(); setStyle(currentGestureBarStyle, BAR_GESTURE_BAR); setStyle(currentStatusBarStyle, BAR_STATUS_BAR); ViewCompat.requestApplyInsets((View) getBridge().getWebView().getParent()); }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java` around lines 166 - 182, The window initialization block duplicated in handleOnResume() and initSystemBars() should be extracted into a single private helper (e.g., configureWindowFlags(Window window) or setupSystemBarWindowFlags(Window window)); move the logic that calls WindowCompat.setDecorFitsSystemWindows(window, false), sets transparent navigation bar for older SDKs and toggles navigationBarContrastEnforced for Q+, into that helper and replace the duplicated blocks with calls to it from both handleOnResume() and initSystemBars(), preserving current Build.VERSION.SDK_INT checks and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java`:
- Around line 353-359: The show logic in SystemBars.java incorrectly leaves
navBarVisible false when calling
windowInsetsControllerCompat.show(WindowInsetsCompat.Type.systemBars()) for
BAR_STATUS_BAR because systemBars() reveals both status and navigation bars;
update the navBarVisible state when showing systemBars() (set navBarVisible =
true) or change the BAR_STATUS_BAR branch to call
windowInsetsControllerCompat.show(WindowInsetsCompat.Type.statusBars()) instead
so the navBarVisible flag remains correct for getNavBarHeightFromResources() and
subsequent safe-area calculations; adjust the conditional handling around
BAR_STATUS_BAR, BAR_GESTURE_BAR and the navBarVisible variable in the same
method to keep hide and show paths symmetric.
---
Nitpick comments:
In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java`:
- Around line 166-182: The window initialization block duplicated in
handleOnResume() and initSystemBars() should be extracted into a single private
helper (e.g., configureWindowFlags(Window window) or
setupSystemBarWindowFlags(Window window)); move the logic that calls
WindowCompat.setDecorFitsSystemWindows(window, false), sets transparent
navigation bar for older SDKs and toggles navigationBarContrastEnforced for Q+,
into that helper and replace the duplicated blocks with calls to it from both
handleOnResume() and initSystemBars(), preserving current Build.VERSION.SDK_INT
checks and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12280262-28ba-4398-9ac2-b99f1d39613a
📒 Files selected for processing (1)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java
RobinWitch
left a comment
There was a problem hiding this comment.
Requesting changes because the required lint job is failing on formatting in the file changed by this PR.
The lint check gets through eslint and then fails at the repository's Prettier check:
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:1:1: error: Line needs to be added (prettier/insert)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:199:24: error: Code style (prettier/format)
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:8184:1: error: Line needs to be added (prettier/insert)
Please run the configured Prettier command on SystemBars.java before merging; as-is the PR blocks the downstream test jobs because lint never completes.
Upstream PR Sync
This PR syncs changes from an external contributor's PR on the official Capacitor repository.
Original PR
Automation
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit
New Features
Bug Fixes